-
Notifications
You must be signed in to change notification settings - Fork 129
[BE-1876] Integrate nixpkgs plugin #1156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
SummaryIntegrated nixpkgs plugin support for faster Ruby installations in mise tool provider. Added backend-prefixed tool request handling, conditional nixpkgs backend usage based on BITRISE_TOOLSETUP_FAST_INSTALL environment variable, and updated version resolution to work with backend-prefixed tool names. Walkthrough
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 2
661ecec to
a7e38f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
0b3c71e to
79f7d2a
Compare
integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go
Outdated
Show resolved
Hide resolved
79f7d2a to
9db8c89
Compare
- Do not clone + link the plugin, use the `mise plugin install ID URL` command instead - Move as much nix plugin specific logic to install.go as possible
MiseExecutor as an interface was created for testing, but I relized this is redundant and ExecEnv should be mockable instead.
6bf3ff2 to
59c8133
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR integrates a nixpkgs plugin for mise to enable faster tool installation for Ruby, with fallback to the core plugin. The changes refactor the mise execution environment into an interface to support testing, add logic to conditionally use the nixpkgs backend, and temporarily force edge stack detection for integration testing.
Key changes:
- Refactored
ExecEnvfrom a struct to an interface withMiseExecEnvimplementation to support testing and dependency injection - Added nixpkgs plugin integration with conditional logic to fall back to core plugins when nixpkgs is unavailable or versions don't match
- Temporarily hardcoded
isEdgeStack()to return true for testing purposes
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| toolprovider/mise/execenv/execenv.go | Converted ExecEnv to interface with MiseExecEnv implementation, exposing InstallDir as a method |
| toolprovider/mise/helpers_test.go | Added fakeExecEnv test helper implementing ExecEnv interface (replaces mockMiseExecutor) |
| toolprovider/mise/install.go | Added nixpkgs integration logic with canBeInstalledWithNix and installRequest functions |
| toolprovider/mise/install_plugin.go | Updated log statement prefix to [TOOLPROVIDER] |
| toolprovider/mise/install_test.go | Added tests for nixpkgs integration and updated existing tests to use fakeExecEnv |
| toolprovider/mise/mise.go | Updated to use new MiseExecEnv constructor, temporarily hardcoded isEdgeStack to return true |
| toolprovider/mise/nixpkgs/nixpkgs.go | New package with nixpkgs backend decision logic and nix availability check |
| toolprovider/mise/resolve.go | Updated to use ExecEnv interface, removed MiseExecutor interface, renamed variables for clarity |
| toolprovider/mise/resolve_test.go | Refactored tests to use fakeExecEnv instead of mockMiseExecutor |
| toolprovider/asdf/install_plugin.go | Updated log statement prefix to [TOOLPROVIDER] |
| integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go | Added integration tests for nixpkgs Ruby installation |
| bitrise.yml | Temporarily updated to use edge stack for integration tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go
Outdated
Show resolved
Hide resolved
toolprovider/mise/mise.go
Outdated
| } | ||
| log.Debugf("Mise: Stack is edge: %s", isEdge) | ||
| return | ||
| // if stack, variablePresent := os.LookupEnv("BITRISEIO_STACK_ID"); variablePresent && strings.Contains(stack, "edge") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented this out because this doesn't catch the Linux edge stack, so we install a too old Mise version.
We could bump our pinned stable Mise version, the unstable Mise version got enough testing on macOS edge stacks I think.
Checklist
README.mdis updated with the changes (if needed)Version
Requires a MAJOR/MINOR/PATCH version update
Context
Changes
Investigation details
Decisions